Skip to content
This repository was archived by the owner on Oct 10, 2025. It is now read-only.

Conversation

@vyredo
Copy link

@vyredo vyredo commented Mar 22, 2025

What kind of change does this PR introduce?

This PR move logic for Lock from the GoTrueClient class by decomposing it into smaller, more focused class. Currently, GoTrueClient is a monolithic class containing several responsibilities including DOM tab visibility, lock management, session/token handling, and more. By grouping related logic into separate classes will improves code organization, maintainability, and testability.

What is the current behavior?

Changes are made without introducing big changes to the logic.
Key optimizations include eliminating redundant await operations and replacing Promise.all with Promise.allSettled to ensure all promises complete execution rather than terminating on the first error.

old logic

try {
  this.lockAcquired = true

  const result = fn()

  this.pendingInLock.push(
    (async () => {
      try {
        await result
      } catch (e: any) {
        // we just care if it finished
      }
    })()
  )

  await result

  // keep draining the queue until there's nothing to wait on
  while (this.pendingInLock.length) {
    const waitOn = [...this.pendingInLock]

    await Promise.all(waitOn)

    this.pendingInLock.splice(0, waitOn.length)
  }

new logic

try {
  this.lockAcquired = true
  const result = fn()

  // make sure fn is the last for this batch
  this.pendingInLock.push(result)
  await this._drainPendingQueue()

  // result have already completed, just unwrap the promise now.
  return await result
}

What is the new behavior?

The new implementation maintains functional equivalence with the original code while providing a cleaner architecture. The refactored code:

  1. Improves separation of concerns by extracting related functionality into dedicated classes
  2. Simplifies the promise handling logic and extra method for queue draining mechanism
  3. Ensuring all promises complete regardless of individual failures

@vyredo vyredo changed the title refactor: Move Lock-related logic to LockManager class feat: Move Lock-related logic to LockManager class Mar 23, 2025
- Improve code organization and testability
- Optimize promise handling with Promise.allSettled
- Simplify lock and queue management logic
- Maintain existing functionality with minor improvements
@vyredo vyredo force-pushed the vyredo/separate-lock-logic branch from 56c5430 to adf971b Compare March 23, 2025 02:17
- Replace direct access to pendingInLock array with _drainPendingQueue method
@mandarini
Copy link
Contributor

Hi @vyredo ! Thanks for the contribution and your patience.

This repository is deprecated and has moved to the new Supabase JS monorepo. I’m going to close it to keep the old repo tidy, before archiving.

If you believe this change is still needed, please open a new PR in the monorepo and include a link back to this thread for context:

Note: This repository is now archived, but you can still see your work, and if needed, copy it over to the new repo. No work is lost!

@mandarini mandarini closed this Oct 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants